perf(multichain): batch OnAsset* requests
#6886
Conversation
danroc
force-pushed
the
dr/batch-rate-conversion
branch
from
a706278 to
468843a
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@metamaskbot publish-preview |
danroc
added
the
area-performance
| // Apply these updated rates to controller state | ||
| this.#applyUpdatedRates(rates); | ||
| for (const asset of this.#getAssetsForAccount(account.id)) { | ||
| assetToSnapID.set(asset, snapId as SnapId); |
There was a problem hiding this comment.
Maybe simpler to construct the Snap ID -> Asset List mapping here instead of constructing this mapping and then constructing the reverse mapping in the next function call?
There was a problem hiding this comment.
I did in two steps mainly because it also dedupes the assets. If I build the snapId -> assets[] mapping here I need to do another pass to remove the deplicates of each entry.
There was a problem hiding this comment.
We could use a Snap ID -> Asset Set for that perhaps? Up to you!
...es/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts
Outdated
Show resolved
Hide resolved
...es/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (assets?.length === 0) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Should we keep this? So we don't create sets for Snaps that don't have any assets
There was a problem hiding this comment.
Indeed, I added some checks to prevent adding empty sets to the map, or calling the Snap when the list of assets is empty.
2 participants
Explanation
Currently,
OnAssetConversionandOnAssetMarketDatarequests are batched per account, which can lead to duplicate requests when an asset is present in multiple accounts.This PR resolves this issue by batching the requests per Snap for all assets managed by each Snap.
References
Checklist
Note
Batch
OnAssetsConversionandOnAssetsMarketDataper Snap across accounts with deduplicated assets; refactor controller, update tests, and note in changelog.#addAssetsToSnapIdMap, deduplicating assets before calling Snaps.#getConversionRates,#getMarketData, and consolidated#getUpdatedRatesForcombining both; added#getCaipCurrentCurrency.#handleSnapRequestwith typed overloads; state merge via#applyUpdatedRatesnow keyed byCaipAssetType.updateAssetsRatesand#updateAssetsRatesForNewAssetsnow build a Snap‑ID→assets map and issue a single batched call per Snap.swift:0/iso4217:USD); add multi‑Snap scenarios and revised spies/mocks.OnAssetConversionandOnAssetsMarketDatarequests to non‑EVM account Snaps.Written by Cursor Bugbot for commit fd1be26. This will update automatically on new commits. Configure here.